fix: Disable httr2 for shinyapps.io bundle uploads#1298
fix: Disable httr2 for shinyapps.io bundle uploads#1298
Conversation
The shinyapps.io API returns a 303 redirect during updateBundleStatus, and httr2 drops the request body on redirect while keeping the X-Content-Checksum header, causing a "bad checksum" HTTP 400 error. Work around this by using the legacy libcurl backend for the entire shinyapps bundle upload flow. Fixes #1297 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Verification: With this fix bundle deployed successfully``` > rsconnect::deployApp(appDir = "/Users/chaitamacpro/Desktop/shinyappIO/shinyapps-1", appName = "shinyapp-2-1") ── Preparing for deployment ────────────────────────────────────────────────────────────── ✔ Re-deploying "shinyapp-2-1" using "server: shinyapps.io / username: chaitatest" ℹ Looking up application with id "16852238"... ✔ Found application ℹ Bundling 1 file: app.R ℹ Capturing R dependencies ✔ Found 29 dependencies ✔ Created 18,708b bundle ℹ Uploading bundle... ✔ Uploaded bundle with id 11685212 ── Deploying to server ─────────────────────────────────────────────────────────────────── Waiting for task: 1661471050 building: Building image: 14512737 building: Fetching packages building: Installing packages building: Installing files building: Pushing image: 14512737 deploying: Starting instances terminating: Stopping old instances ── Deployment complete ─────────────────────────────────────────────────────────────────── ✔ Successfully deployed to ``` |
There was a problem hiding this comment.
I wonder if instead we could prevent httr2 from following redirects (httr2::req_options(req, followlocation = FALSE)). We already have code to handle redirects ourselves in httpRequestWithBody, we could potentially move this outside the if:
Lines 131 to 146 in 591e652
Move the httr2 workaround from uploadShinyappsBundle to
clientForAccount. The uploadShinyappsBundle-only fix was insufficient
because on.exit() restored rsconnect.httr2 = TRUE before
deployApplication ran. deployApplication also gets a 303 redirect
(to /v1/tasks/{id}), causing a 405 "Method Not Allowed" error.
Disabling httr2 in clientForAccount covers the entire shinyapps.io
deployment flow: bundle upload, deploy, task polling, and terminate.
Fixes #1297
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Thanks, @karawoo, for taking a look! I'm not super familiar with the httr2 internals to be confident making deeper changes to the redirect handling. I've updated the PR with a broader workaround that disables httr2 for all shinyapps.io API calls, and the integration tests pass locally. A couple of things I found while testing:
@nealrichardson I've added the workaround for shinyapps.io and have integration tests passing locally. When we're ready to run them in CI, we'll need someone with admin access to add the shinyapps.io credentials as repository secrets. Also, Kara's suggestion of using |
As currently written, httr2 gets disabled globally in the R session when a shinyapps.io client is set up, which will also affect API calls to Connect and Connect Cloud if done in the same R session after a shinyapps.io client is created. I think this isn't quite what we want. If we want to disable htt2 for POST requests to shinyapps.io across the board, then I think we could add logic here: Line 105 in 591e652 if (isTRUE(getOption("rsconnect.httr2", TRUE)) && !isTRUE(grepl("shinyapps\\.io$", service$host))) { |
|
@karawoo we discussed disabling the httr2 redirect following and keeping the manual logic, but since this is just the behavior of one endpoint on one legacy system, I wasn't sure it was worth keeping that around. @ChaitaC I was thinking of a more narrow setting, similar to what you originally did, but you can probably do this: diff --git a/R/client-shinyapps.R b/R/client-shinyapps.R
index a54a7516..4b068fc9 100644
--- a/R/client-shinyapps.R
+++ b/R/client-shinyapps.R
@@ -73,6 +73,8 @@ shinyAppsClient <- function(service, authInfo) {
json$content_type <- content_type
json$content_length <- content_length
json$checksum <- checksum
+ old <- options(rsconnect.httr2 = FALSE)
+ on.exit(options(old))
POST_JSON(service, authInfo, "/bundles", json)
},and only affect that one request. Did something not work when you had the original commit, such that you needed to make it broader. You can DM me the secret and I can add it to the actions. You can push the test whenever, just make it skipped if the env var is not set, and then we can wire it up with the secret once I add it. |
|
@nealrichardson The first commit wrapped uploadShinyappsBundle(), which covered createBundle, the S3 upload, and updateBundleStatus. Those all succeeded, but deployApplication is called after uploadShinyappsBundle returns, so on.exit() had already restored rsconnect.httr2 = TRUE. deployApplication then hits a 303 redirect and fails: That's why I broadened it in commit 2. (Not the correct call, though) I also tested your suggested fix (wrapping just createBundle). createBundle itself doesn't get a 303 redirect, so the workaround there isn't needed. It fails at updateBundleStatus instead: Through testing different combinations, I narrowed the fix down to 3 functions that need it: updateBundleStatus (400 bad checksum), deployApplication (405, confirmed above), and terminateApplication (405, confirmed by removing its workaround and watching the test fail). Adding options(rsconnect.httr2 = FALSE) + on.exit() to just those 3 works and passes integration tests. That said, Kara's suggested fix at http.R:105 is simpler and more defensive: It covers all shinyapps.io requests in one place without leaking to Connect/Cloud. Happy to go either way — let me know which you'd prefer. |
Move the httr2 workaround from clientForAccount (global session option) to the HTTP dispatch functions in http.R. This disables httr2 for all requests to shinyapps.io hosts without leaking to Connect/Cloud. The shinyapps.io API returns 303 redirects on several POST endpoints (updateBundleStatus, deployApplication, terminateApplication), and httr2 mishandles these redirects, causing "bad checksum" (400) and "Method Not Allowed" (405) errors. Also adds shinyapps.io integration tests that exercise the full deploy and terminate flow. Fixes #1297 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add an if condition to the workflow step so the tests are skipped entirely when SHINYAPPS_SECRET is not set, rather than failing with an error from setup.R skip(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
If we did want to go the route of handling redirects explicitly I think it would just involve adding while (isRedirect(httpResponse$status)) {
service <- redirectService(service, httpResponse$location)
authed_headers <- c(headers, authHeaders(authInfo, "GET", service$path))
resp <- httr2Request(
service,
authInfo,
"GET",
service$path,
authed_headers,
certificate = certificate
)
httpResponse <- httr2_response_to_list(resp)
}after this line, adding Do we think this is worth just adding? It seems like in exchange for maintaining this code we gain the ability to drop httr whenever we're ready, which we can't do if we're relying on it for shinyapps.io. |
Summary
rsconnect.httr2 = FALSEinuploadShinyappsBundle()updateBundleStatus, and httr2 drops the request body on redirect while keeping theX-Content-Checksumheader, causing a "bad checksum" HTTP 400 erroron.exit()Fixes #1297